Use presets to change a form

Question

I intend to populate some fields in a form based on some presets values. The user choose the presets from a list. The following code is working, but as I'm a newbie, I would like to know if I done it the proper way and if it can be improved.

Preset obj:

var obj = {
 "preset_a":{
 "input":{
 "field_a":"value",
 ...
 "field_b":"value",
 },
 "select":{
 ...
 },
 "radio":{
 ...
 }
 },
 "preset_b":{
 "input":{
 "field_a":"value",
 ...
 "field_b":"value",
 },
 "select":{
 ...
 },
 "radio":{
 ...
 }
 }
};

The script:

function applyPresets(preset)
{
 if (typeof obj[preset] != 'undefined')
 {
 if (typeof obj[preset]['input'] != 'undefined')
 {
 jQuery.each(obj[preset]['input'], function( k, v ) {
 jQuery("input#"+ k).val(v);
 });
 }
 if (typeof obj[preset]['select'] != 'undefined')
 {
 jQuery.each(obj[preset]['select'], function( k, v ) {
 jQuery("select#" + k).val(v);
 });
 }
 if (typeof obj[preset]['radio'] != 'undefined')
 {
 jQuery.each(obj[preset]['radio'], function( k, v ) {
 jQuery("fieldset#" + k + " > input[type=radio]").val(v);
 });
 }
 }
 else
 {
 alert('Preset [ ' + preset + ' ] not found!');
 return;
 }
 submitSettings();
}

Answer 1

If the property name has spaces or special characters, there is no need to quote it (and it looks more clear):

var obj = {
 preset_a: {
 input: {
 field_a: "value",
 field_b: "value",
 }
 "i need quotes": "value"
 }
}

I tried to improve the function a little bit. I did it on the fly so i'm pretty sure that something could be wrong. Check it out:

function applyPresets(preset){
 // as Quill said, the return(s) statement has more sense at the top of the method
 if( typeof obj[preset] == 'undefined' ){
 alert('Preset [ ' + preset + ' ] not found!'); 
 return;
 }
 for( var i in obj[preset] ){
 if( obj[preset].hasOwnProperty(i) ) { // .hasOwnProperty() can be used to determine whether an object has the specified property as a direct property of that object, very useful here
 $.each(obj[preset][i], function(k, v){ // since the code seems very similar we can improve it avoiding more loops
 if( i == 'radio' ){ // if the code below doesn't match, we will need this `if`
 $('fieldset#' + k + ' > input[type="radio"]').val(v);
 }
 else $(i + '#' + k).val(v); // while they match we don't need more code
 });
 }
 }
 submitSettings();
}

In summary: We use an in loop through the obj[preset] object. This gives us the name of i, so we can easily discard undefined(s) thanks to the .hasOwnProperty() method. The last step is avoid the loops. The syntax is quite the same so we can work inside only one .each() loop. If the selector changes we can always use the lovely if statement.

I made a jsFiddle with the code if you want to see it clearly.

Answer 2

• You can replace jQuery( with $( in most places, for example: jQuery("input#"+ k): becomes: $("input#" + k), also note you need whitespace between your boolean operators (+)

• However, you shouldn't have whitespace between your parameters like in function( k, v ), which should be: function(k, v)

• Rather than encasing the main contents of the function inside:
  if (typeof obj[preset] != 'undefined'){, you can put the return statement at the beginning of the function, so that if the conditions are met, the function returns, and otherwise, you can just continue on, without needing to be inside an if-loop:

  if (typeof obj[preset] != 'undefined'){
   doStuff();
  } else {
   alert('Preset [ ' + preset + ' ] not found!');
   return;
  }
  

  becomes:

  if (typeof obj[preset] == 'undefined'){
   alert('Preset [ ' + preset + ' ] not found!');
   return;
  }
  doStuff();
  

• I'm not 100% sure on this, but I believe undefined can be expressed without being inside quotes.